8320500: [vectorapi] RISC-V: Optimize vector math operations with SLEEF #21083
Conversation
|
👋 Welcome back mli! A progress list of the required criteria for merging this PR into |
|
Hi @magicus , could you please have a look at the make part? Thanks! |
|
@Hamlin-Li This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 19 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
@Hamlin-Li The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
Thanks for your reviewing! |
src/jdk.incubator.vector/linux/native/libsleef/lib/vector_math_rvv.c
Outdated
Show resolved
Hide resolved
openjdk
bot
removed
the
merge-conflict
|
Thanks everyone for your reviewing! I'll push this later after I finally verify the performance with the latest patch. |
|
/integrate |
|
@Hamlin-Li This pull request has not yet been marked as ready for integration. |
|
@erikj79 @RealFYang Hi, can you help to reapprove the pr? I guess it's because the new "re-review/approve" rule. Thanks! |
|
@luhenry I merged this pr with the master to resolve conflict after your approval, so I think I also needs your reapproval. Thanks |
|
I apologize for bringing in new requests for changes when you were so close to integrating. I understand it must be frustrating. Hopefully, it's a trivial fix and I can re-approve the new version as soon as you commit it. |
|
I've run some jtreg test, and check the generated libsleef.so, it should be good. No further jmh test re-run as seems it's not necessary. |
|
Sorry for the inconvenience. Thanks for your quick re-reviews! /integrate |
|
Going to push as commit 580eb62.
Your commit was automatically rebased without conflicts. |
openjdk
bot
removed
ready
|
@Hamlin-Li Pushed as commit 580eb62. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
|
@Hamlin-Li are you still planning to re-submit the AArch64 backend changes that were in #18605 ? |
|
Yes, I could probably send out for review next week. |
Hi,
Can you help to review this patch?
Thanks!
This patch is based on #20781 which added the sleef source (in particular the generated sleef inline headers). We use sleef api to vectorize the math operations in vector api.
On machine with vector intrinsic support on riscv (e.g. gcc 14+) it will generate libsleef.so with the bridge functions to sleef api, otherwise without the bridge functions.
Test
test/jdk/jdk/incubator/vector
Performance
data on bananapi
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21083/head:pull/21083$ git checkout pull/21083Update a local copy of the PR:
$ git checkout pull/21083$ git pull https://git.openjdk.org/jdk.git pull/21083/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 21083View PR using the GUI difftool:
$ git pr show -t 21083Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21083.diff
Webrev
Link to Webrev Comment